Custom inline cache implementation #60
Conversation
mikejholly
force-pushed
the
mh/next-inline-cache-redux
branch
3 times, most recently
from
a917e12
to
4afd23a
Compare
| Cache []byte `json:"moby.buildkit.cache.v0"` | ||
| History []struct { | ||
| Cache []byte `json:"moby.buildkit.cache.v0"` | ||
| EarthlyInlineCache []byte `json:"earthly.inlinecache.v0"` |
There was a problem hiding this comment.
This field is added to an image's "config" and can be viewed using something like crane config. The map of cache keys to registry layer digests is stored here.
| @@ -53,7 +53,7 @@ func (w *runcExecutor) monitorContainerStats(ctx context.Context, id string, sam | |||
| for { | |||
| select { | |||
| case <-ctx.Done(): | |||
| bklog.G(ctx).Infof("stats collection context done: %v", ctx.Err()) | |||
| bklog.G(ctx).Debugf("stats collection context done: %v", ctx.Err()) | |||
There was a problem hiding this comment.
This helps with log noise.
| if err != nil { | ||
| return nil, err | ||
| } | ||
| m["earthly.inlinecache.v0"] = dt |
There was a problem hiding this comment.
This is where the cache key to layer digest data is added.
solver/llbsolver/bridge.go
Outdated
| } | ||
|
|
||
| err := inBuilderContext(ctx, b.builder, "earthly inline cache", "", func(ctx context.Context, g session.Group) error { | ||
| inlineCacheRemotes, err = registry.EarthlyInlineCacheRemotes(ctx, b.sm, w, b.registryHosts, g, cacheImport.Attrs) |
There was a problem hiding this comment.
This is where remote manifests (including cache relation info) are fetched for the inline cache source.
| return nil, err | ||
| } | ||
|
|
||
| sources := worker.NewCombinedResultSource( |
There was a problem hiding this comment.
Here's a new abstraction that allows the simple solver to load from a set of result sources. The inline cache remote source is specified here.
| // } | ||
|
|
||
| if hasInlineCacheExporter(exp.CacheExporters) { | ||
| meta, err := earthlyInlineCache(ctx, j, e, cached) |
There was a problem hiding this comment.
Here, we determine if inline caching is enabled. If so, the cache relationship map is added to the image metadata before the image is passed to the Earthly exporter.
|
|
||
| // Hijack the CacheKey type in order to export a reference from the new cache key to the ref ID. | ||
| expKeys = append(expKeys, ExportableCacheKey{ | ||
| CacheKey: &CacheKey{ |
There was a problem hiding this comment.
In the BuildKit remote caching approach, a complex chain of cache keys are added to ExportableCacheKey. Here, I'm able to stick with the return type (needed by the caller & caller parent) while using a simplified one-to-one cache key to worker ref ID relation.
| mu sync.Mutex | ||
| } | ||
|
|
||
| type cacheMapDep struct { | ||
| selector string | ||
| computed string | ||
| selector digest.Digest |
There was a problem hiding this comment.
I switched to digest.Digest in a few places because using strings were leading to annoying conversions. They're just strings with some helper methods.
mikejholly
force-pushed
the
mh/next-inline-cache-redux
branch
from
4afd23a
to
9ea0325
Compare
| return &inMemCache{cache: map[string]Result{}} | ||
| // RefIDStore uses a BoltDB database to store links from computed cache keys to | ||
| // worker ref IDs. | ||
| type RefIDStore struct { |
There was a problem hiding this comment.
I split the ref ID storage out from the result loader so that the former can be used in the solver & worker/simple.go.
mikejholly
force-pushed
the
mh/next-inline-cache-redux
branch
4 times, most recently
from
750aa06
to
bab4556
Compare
| } | ||
|
|
||
| layerDone := progress.OneOff(ctx, fmt.Sprintf("inferred cache manifest type: %s", manifestType)) | ||
| layerDone(nil) |
There was a problem hiding this comment.
I'm guessing this is intentional since I see it done in cache/remotecache/azblob/importer.go, but as someone who's new to the OneOff, this seems confusing. but since it's a pattern that's already used in buildkit maybe it's fine.
There was a problem hiding this comment.
Much of this code was taken from the preexisting inline cache code: https://github.com/earthly/buildkit/blob/earthly-main/cache/remotecache/import.go#L58
solver/llbsolver/bridge.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| bklog.G(ctx).Warnf("failed to import inline cache: %v", err) |
There was a problem hiding this comment.
did you intentionally mean to drop the return here?
There was a problem hiding this comment.
Yes, it's probably not necessary to fail the build on an error. But now that you mention it, users may want to know. What do you think?
There was a problem hiding this comment.
Updated to return the error and fail the build. It's likely useful to know when a network error has occurred.
solver/llbsolver/bridge.go
Outdated
| } | ||
| } | ||
|
|
||
| // MH: Old-school remote & inline cache imports disabled. |
There was a problem hiding this comment.
should we just delete this instead?
mikejholly
force-pushed
the
mh/next-inline-cache-redux
branch
from
b88c5bd
to
abb84c9
Compare
This PR includes a new & simplified inline cache implementation that's compatible with the simple solver.
See comments in PR.
Earthly tests: earthly/earthly#4112